-
-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat incentive offers #325
Conversation
@github-actions[bot] is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
WalkthroughThe changes in this pull request primarily involve updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
frontend/src/App.jsx (3)
3-4
: LGTM! Consider optimizing React import.The imports for React and useState are correct and necessary for the component's functionality.
For newer versions of React (17+), you can omit the explicit React import unless you're using JSX runtime:
-import React from 'react'; import { useState } from 'react';
This change would slightly reduce bundle size.
12-12
: LGTM! Consider user experience for returning visitors.The state variable for controlling modal visibility is correctly implemented.
Consider enhancing the user experience by not showing the modal every time for returning users. You could use local storage to track if the user has seen the offer recently:
const [showModal, setShowModal] = useState(() => { const lastShown = localStorage.getItem('lastOfferShown'); const showAgain = !lastShown || Date.now() - parseInt(lastShown) > 24 * 60 * 60 * 1000; // 24 hours if (showAgain) { localStorage.setItem('lastOfferShown', Date.now().toString()); } return showAgain; });This approach would show the offer once per day, improving the balance between engagement and user experience.
16-16
: LGTM! Consider adding an onOpen callback for analytics.The Offers component is correctly implemented with proper props for visibility control and closing functionality.
To further align with the PR objectives of improving user engagement, consider adding an onOpen callback to track when users view the offer:
<Offers isVisible={showModal} onClose={() => setShowModal(false)} onOpen={() => { // Add analytics tracking here console.log('Offer modal opened'); }} />This would allow you to gather data on how often users are seeing the offers, which could be valuable for future improvements.
frontend/src/components/Shared/Offers.jsx (2)
8-18
: LGTM: Good use of useEffect for managing body overflow.The useEffect hook is well-implemented to handle body overflow based on modal visibility. This prevents background scrolling when the modal is open, which is a good UX practice.
Consider extracting the overflow styles into constants for better readability:
const HIDDEN_OVERFLOW = 'hidden'; const AUTO_OVERFLOW = 'auto'; useEffect(() => { document.body.style.overflow = isVisible ? HIDDEN_OVERFLOW : AUTO_OVERFLOW; return () => { document.body.style.overflow = AUTO_OVERFLOW; }; }, [isVisible]);
28-83
: LGTM: Well-structured modal with appealing design.The modal structure and content are well-organized and visually appealing. The use of Tailwind CSS classes provides responsive styling, and the layout is clear with distinct sections for the image and text.
Consider adding
aria-label
attributes to the buttons for improved accessibility:- <button + <button + aria-label="Close offer modal" className="absolute top-2 right-2 text-black text-xl" onClick={() => onClose()} > <RxCross1 color='black' /> </button> // ... (other code) - <button + <button + aria-label="Dismiss offer" className="bg-[#f5deb3] text-black py-2 px-4 rounded-lg hover:bg-[#eed19b] transition" onClick={() => onClose()} > No thanks </button> - <button + <button + aria-label="Accept offer and go to menu" className="bg-[#d2b48c] text-black py-2 px-4 rounded-lg hover:bg-[#c8a682] transition" onClick={handleTakeMeThere} > Take Me There </button>README.md (2)
Line range hint
225-405
: Consider maintaining a historical record of all contributors.While updating the contributors list is necessary to reflect current active contributors, it's important to ensure that all contributions are properly recognized. Consider implementing one of the following suggestions:
- Maintain a separate "Hall of Fame" or "Past Contributors" section in the README or a separate CONTRIBUTORS.md file to acknowledge all past contributors.
- Use a tool like the All Contributors bot to automatically track and display all contributors, both past and present.
This approach ensures that every contribution is valued and remembered, which is crucial for open-source projects.
Would you like assistance in setting up an automated solution for managing contributors, such as the All Contributors bot?
🧰 Tools
🪛 Markdownlint
370-370: Column: 1
Hard tabs(MD010, no-hard-tabs)
371-371: Column: 1
Hard tabs(MD010, no-hard-tabs)
238-239
: Ensure consistent indentation throughout the document.Markdownlint has flagged the use of hard tabs in several places, particularly around lines 238-239 and 370-371. While not strictly incorrect, inconsistent indentation can affect readability and cause issues with some Markdown parsers.
Consider replacing all hard tabs with spaces (typically 2 or 4 spaces per indentation level) throughout the document for consistency. You can use the following command to convert tabs to spaces:
sed -i 's/\t/ /g' README.md
This will replace all tabs with 4 spaces. Adjust the number of spaces as needed to match your project's style guide.
Also applies to: 370-371
🧰 Tools
🪛 Markdownlint
238-238: Column: 1
Hard tabs(MD010, no-hard-tabs)
239-239: Column: 1
Hard tabs(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- README.md (8 hunks)
- frontend/src/App.jsx (2 hunks)
- frontend/src/components/Shared/Navbar.jsx (1 hunks)
- frontend/src/components/Shared/Offers.jsx (1 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
238-238: Column: 1
Hard tabs(MD010, no-hard-tabs)
239-239: Column: 1
Hard tabs(MD010, no-hard-tabs)
370-370: Column: 1
Hard tabs(MD010, no-hard-tabs)
371-371: Column: 1
Hard tabs(MD010, no-hard-tabs)
🔇 Additional comments (9)
frontend/src/App.jsx (2)
10-10
: LGTM! Offers component imported correctly.The import of the Offers component is correct and aligns with the PR objective of introducing an offer modal.
Line range hint
1-25
: Overall, great implementation of the offer modal feature!The changes in this file successfully introduce the offer modal as per the PR objectives. The implementation is clean and functional. The suggestions provided aim to further enhance user experience and provide better analytics for future improvements.
Great job on this feature! It should effectively help in improving user engagement with the café's menu.
frontend/src/components/Shared/Offers.jsx (4)
1-6
: LGTM: Imports and component declaration are well-structured.The imports and component declaration are appropriate for the intended functionality. The use of
useNavigate
from react-router-dom is a good choice for handling navigation within the component.
20-26
: LGTM: Efficient conditional rendering and navigation handling.The conditional rendering and navigation handling are well-implemented. Returning null when the modal is not visible is an efficient approach. The
handleTakeMeThere
function correctly closes the modal before navigation, ensuring a smooth user experience.
87-87
: LGTM: Correct component export.The Offers component is correctly exported as the default export, which is the appropriate method for exporting a single component from a file.
1-87
: Overall assessment: Well-implemented Offers component.The Offers component successfully implements the promotional modal as described in the PR objectives. It effectively addresses the goal of enhancing user engagement and driving traffic to the menu page. The component is well-structured, uses React hooks appropriately, and follows good practices for modal design and user interaction.
Key strengths:
- Responsive and visually appealing design using Tailwind CSS.
- Proper management of body overflow for better user experience.
- Clear navigation handling with react-router-dom.
- Efficient conditional rendering.
The minor suggestions for improvement (constants for overflow styles and accessibility attributes) will further enhance the component's maintainability and accessibility.
This implementation aligns well with the objectives of improving customer retention and increasing website traffic through incentive strategies.
frontend/src/components/Shared/Navbar.jsx (1)
74-77
: Verify z-index change impact and approve conditional styling.The z-index change from z-50 to z-20 might affect the stacking order of the navbar. Ensure this doesn't cause any unintended overlapping issues with other elements, especially modals or dropdowns.
The addition of conditional classes for background color, text color, and shadow based on the scroll state is a good improvement for visual feedback.
To verify the z-index change impact, run the following script:
Review the results to ensure no critical elements are affected by the z-index change.
✅ Verification successful
z-index Change Verified and Conditional Styling Approved
The z-index change from z-50 to z-20 does not introduce any stacking conflicts, ensuring the navbar remains correctly layered. The added conditional classes for background color, text color, and shadow based on the scroll state effectively enhance visual feedback.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential z-index conflicts # Search for z-index values higher than 20 in CSS/SCSS files echo "Searching for z-index values higher than 20:" rg --type css --type scss 'z-index:\s*([2-9][1-9]|[3-9]\d|\d{3,})' -g '!node_modules' # Search for elements that might need to appear above the navbar echo "Searching for potential overlapping elements:" rg --type jsx --type tsx '(Modal|Dropdown|Popover|Tooltip)' -g '!node_modules'Length of output: 425
Script:
#!/bin/bash # Description: Check for potential z-index conflicts # Search for z-index values higher than 20 in CSS/SCSS files echo "Searching for z-index values higher than 20:" rg 'z-index:\s*([2-9][1-9]|[3-9]\d|\d{3,})' -g '*.css' -g '*.scss' --ignore-file node_modules # Search for elements that might need to appear above the navbar echo "Searching for potential overlapping elements:" rg '(Modal|Dropdown|Popover|Tooltip)' -g '*.jsx' -g '*.tsx' --ignore-file node_modulesLength of output: 1912
Script:
#!/bin/bash # Description: Check for potential z-index conflicts # Search for z-index values higher than 20 in CSS/SCSS files echo "Searching for z-index values higher than 20:" rg 'z-index:\s*([2-9][1-9]|[3-9]\d|\d{3,})' -g '*.css' -g '*.scss' --glob '!node_modules/**' # Search for elements that might need to appear above the navbar echo "Searching for potential overlapping elements:" rg '(Modal|Dropdown|Popover|Tooltip)' -g '*.jsx' -g '*.tsx' --glob '!node_modules/**'Length of output: 1804
README.md (2)
Line range hint
1-405
: Overall, the changes look good with some minor suggestions for improvement.The updates to the contributors list appear to be the main focus of these changes. The modifications seem appropriate, but please ensure they align with your project's contribution guidelines. Consider the suggestions for maintaining a historical record of contributors and improving indentation consistency.
These changes will help keep your README up-to-date and maintain a welcoming environment for all contributors, past and present.
🧰 Tools
🪛 Markdownlint
370-370: Column: 1
Hard tabs(MD010, no-hard-tabs)
371-371: Column: 1
Hard tabs(MD010, no-hard-tabs)
225-228
: Verify intentional updates to the contributors list.The contributors section has undergone several changes, including updates to names, GitHub usernames, and the removal of some entries. Please confirm that these modifications are intentional and align with the project's contribution guidelines.
Some notable changes include:
- Updated entry for Ashwini_ab (line 225-228)
- Updated entry for Sajal Batra (line 248-251)
- Updated entry for Abhijit Motekar (line 262-265)
- Updated entry for Mahera Nayan (previously Abhijit Motekar) (line 269-272)
- Updated entry for Sawan kushwah (line 299-302)
- Removed entry for Aman Yadav (line 312-318)
- Updated entry for Jay shah (line 342-348)
- Updated entry for Nilanchal (line 363-369)
- Added entry for dev129 (line 399-405)
To ensure these changes are correct, you may want to cross-reference with recent pull requests or contributor activity. Here's a script to help verify recent contributor activity:
Also applies to: 238-239, 248-251, 262-265, 269-272, 299-302, 312-318, 342-348, 363-369, 399-405
✅ Verification successful
Contributor list updates are intentional and align with recent activity.
The changes to the contributors section reflect the current active contributors based on recent commit activity.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify recent contributor activity # List recent commits and their authors echo "Recent commits and authors:" git log --pretty=format:"%h - %an, %ar : %s" -n 10 # List contributors who have been active in the last month echo -e "\nContributors active in the last month:" git shortlog -sn --since="1 month ago"Length of output: 1162
This PR has been automatically closed due to inactivity from the owner for 3 days. |
PR Title: Add Offer Modal with Highlighted CTA Button (#196)
Description:
This PR addresses issue #196 by introducing an engaging offer modal aimed at encouraging users to explore our menu and learn more about the café. The modal showcases a clear and compelling call-to-action (CTA) with the following key features:
This design aims to capture user attention and provide an interactive way to explore ongoing promotions, ultimately enhancing user engagement with our café’s offerings.
Changes Included:
Impact:
This feature not only directs users to the menu but also encourages deeper engagement with the website, potentially boosting traffic and conversions for our promotional offers.
Please review the changes and provide any feedback!
Closes #196.
New Features:
Updates:
Summary by CodeRabbit
Release Notes